From 8a19eb743741e5ff6a9ef899b632b66070b668b7 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 15 Aug 2014 17:40:08 -0700 Subject: [PATCH] Factor in .gitignore for build cmd freshness When testing the freshness of a build command, the fingerprint of an entire package is generated. The current method of fingerprinting a path source is to just stat() the entire tree and look at mtimes. This works fine when the build command places *all* output in the build directory. Some systems, like autotools, place *most* output in the build directory and some output in the source directory, however (see spidermonkey). In this case the coarse-grained "consider all files in a directory as input" is too painful for the build command as the package will forever be rebuilt. This commit adds support for filtering the list of files in a directory considered to be part of a package by using `git ls-files`. This git-specific logic can be extended to other VCSs when cargo grows support for them. Closes #379 --- src/cargo/sources/path.rs | 105 ++++++++++++++++++++------- tests/test_cargo_compile_git_deps.rs | 41 +++++++++++ 2 files changed, 118 insertions(+), 28 deletions(-) diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index 8a6622d62..c7d0e7d51 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -5,7 +5,7 @@ use std::io::fs; use core::{Package, PackageId, Summary, SourceId, Source, Dependency, Registry}; use ops; -use util::{CargoResult, internal, internal_error}; +use util::{CargoResult, internal, internal_error, process}; pub struct PathSource { id: SourceId, @@ -57,6 +57,74 @@ impl PathSource { ops::read_packages(&self.path, &self.id) } } + + /// List all files relevant to building this package inside this source. + /// + /// This function will use the appropriate methods to determine what is the + /// set of files underneath this source's directory which are relevant for + /// building `pkg`. + /// + /// The basic assumption of this method is that all files in the directory + /// are relevant for building this package, but it also contains logic to + /// use other methods like .gitignore to filter the list of files. + pub fn list_files(&self, pkg: &Package) -> CargoResult> { + // TODO: add an `excludes` section to the manifest which is another way + // to filter files out of this set that is returned. + return if self.path.join(".git").exists() { + self.list_files_git(pkg) + } else { + self.list_files_walk(pkg) + }; + + fn list_files_git(&self, pkg: &Package) -> CargoResult> { + let cwd = pkg.get_manifest_path().dir_path(); + let mut cmd = process("git").cwd(cwd.clone()); + cmd = cmd.arg("ls-files").arg("-z"); + + // Filter out all other packages with a filter directive + for pkg in self.packages.iter().filter(|p| *p != pkg) { + if cwd.is_ancestor_of(pkg.get_manifest_path()) { + let filter = pkg.get_manifest_path().dir_path() + .path_relative_from(&self.path).unwrap(); + cmd = cmd.arg("-x").arg(filter); + } + } + + log!(5, "listing git files with: {}", cmd); + let output = try!(cmd.arg(".").exec_with_output()); + let output = output.output.as_slice(); + Ok(output.split(|x| *x == 0).map(Path::new).collect()) + } + + fn list_files_walk(&self, pkg: &Package) -> CargoResult> { + let mut ret = Vec::new(); + for pkg in self.packages.iter().filter(|p| *p == pkg) { + let loc = pkg.get_manifest_path().dir_path(); + try!(walk(&loc, &mut ret, true)); + } + return Ok(ret); + + fn walk(path: &Path, ret: &mut Vec, + is_root: bool) -> CargoResult<()> { + if !path.is_dir() { + ret.push(path.clone()); + return Ok(()) + } + // Don't recurse into any sub-packages that we have + if !is_root && path.join("Cargo.toml").exists() { return Ok(()) } + for dir in try!(fs::readdir(path)).iter() { + match (is_root, dir.filename_str()) { + (_, Some(".git")) | + (true, Some("target")) | + (true, Some("Cargo.lock")) => continue, + _ => {} + } + try!(walk(dir, ret, false)); + } + return Ok(()) + } + } + } } impl Show for PathSource { @@ -105,33 +173,14 @@ impl Source for PathSource { } let mut max = 0; - - for pkg in self.packages.iter().filter(|p| *p == pkg) { - let loc = pkg.get_manifest_path().dir_path(); - max = cmp::max(max, try!(walk(&loc, true))); - } - - return Ok(max.to_string()); - - fn walk(path: &Path, is_root: bool) -> CargoResult { - if !path.is_dir() { - // An fs::stat error here is either because path is a - // broken symlink, a permissions error, or a race - // condition where this path was rm'ed - either way, - // we can ignore the error and treat the path's mtime - // as 0. - return Ok(fs::stat(path).map(|s| s.modified).unwrap_or(0)) - } - // Don't recurse into any sub-packages that we have - if !is_root && path.join("Cargo.toml").exists() { return Ok(0) } - - let mut max = 0; - for dir in try!(fs::readdir(path)).iter() { - if is_root && dir.filename_str() == Some("target") { continue } - if is_root && dir.filename_str() == Some("Cargo.lock") { continue } - max = cmp::max(max, try!(walk(dir, false))); - } - return Ok(max) + for file in try!(self.list_files(pkg)).iter() { + // An fs::stat error here is either because path is a + // broken symlink, a permissions error, or a race + // condition where this path was rm'ed - either way, + // we can ignore the error and treat the path's mtime + // as 0. + max = cmp::max(max, file.stat().map(|s| s.modified).unwrap_or(0)); } + Ok(max.to_string()) } } diff --git a/tests/test_cargo_compile_git_deps.rs b/tests/test_cargo_compile_git_deps.rs index e69cc0e75..cfb4c87d0 100644 --- a/tests/test_cargo_compile_git_deps.rs +++ b/tests/test_cargo_compile_git_deps.rs @@ -1011,3 +1011,44 @@ test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured ", compiling = COMPILING, url = p.url(), running = RUNNING, bar = p2.url()))); }) + +test!(git_build_cmd_freshness { + let foo = git_repo("foo", |project| { + project.file("Cargo.toml", r#" + [package] + name = "foo" + version = "0.0.0" + authors = [] + build = "true" + "#) + .file("src/lib.rs", "pub fn bar() -> int { 1 }") + .file(".gitignore", " + src/bar.rs + Cargo.lock + ") + }).assert(); + foo.root().move_into_the_past().assert(); + + assert_that(foo.process(cargo_dir().join("cargo-build")), + execs().with_status(0) + .with_stdout(format!("\ +{compiling} foo v0.0.0 ({url}) +", compiling = COMPILING, url = foo.url()))); + + // Smoke test to make sure it doesn't compile again + println!("first pass"); + assert_that(foo.process(cargo_dir().join("cargo-build")), + execs().with_status(0) + .with_stdout(format!("\ +{fresh} foo v0.0.0 ({url}) +", fresh = FRESH, url = foo.url()))); + + // Modify an ignored file and make sure we don't rebuild + println!("second pass"); + File::create(&foo.root().join("src/bar.rs")).assert(); + assert_that(foo.process(cargo_dir().join("cargo-build")), + execs().with_status(0) + .with_stdout(format!("\ +{fresh} foo v0.0.0 ({url}) +", fresh = FRESH, url = foo.url()))); +}) -- 2.30.2